test: Add attestation marshal tests#4253
Conversation
gmlewis
changed the title
| @@ -0,0 +1,71 @@ | |||
| // Copyright 2024 The go-github AUTHORS. All rights reserved. | |||
There was a problem hiding this comment.
| // Copyright 2024 The go-github AUTHORS. All rights reserved. | |
| // Copyright 2026 The go-github AUTHORS. All rights reserved. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4253 +/- ##
=======================================
Coverage 97.48% 97.48%
=======================================
Files 190 190
Lines 19178 19178
=======================================
Hits 18695 18695
Misses 268 268
Partials 215 215 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks, updated the new test file copyright year to 2026 and reran the focused marshal test plus diff check. |
| func TestAttestation_Marshal(t *testing.T) { | ||
| testJSONMarshalOnly(t, &Attestation{}, `{"bundle": null, "repository_id": 0}`) |
There was a problem hiding this comment.
| func TestAttestation_Marshal(t *testing.T) { | |
| testJSONMarshalOnly(t, &Attestation{}, `{"bundle": null, "repository_id": 0}`) | |
| func TestAttestation_Marshal(t *testing.T) { | |
| t.Parallel() | |
| testJSONMarshalOnly(t, &Attestation{}, `{"bundle": null, "repository_id": 0}`) |
| func TestAttestationsResponse_Marshal(t *testing.T) { | ||
| testJSONMarshal(t, &AttestationsResponse{}, `{"attestations": null}`) |
There was a problem hiding this comment.
| func TestAttestationsResponse_Marshal(t *testing.T) { | |
| testJSONMarshal(t, &AttestationsResponse{}, `{"attestations": null}`) | |
| func TestAttestationsResponse_Marshal(t *testing.T) { | |
| t.Parallel() | |
| testJSONMarshal(t, &AttestationsResponse{}, `{"attestations": null}`) |
gmlewis
changed the title
There was a problem hiding this comment.
No need for Attestation and AttestationsResponse JSON marshal tests. We should only add tests for types with custom MarshalJSON or UnmarshalJSON implementations. For example:
go-github/github/repos_environments.go
Line 156 in d40cb97
Line 205 in d40cb97
@gmlewis what do you think? These tests, generated by an LLM, don't actually validate anything meaningful and only add maintenance overhead.
4 participants
Problem
Artifact attestation payload types are shared by organization, repository, and user attestation APIs, but the shared resource structs did not have dedicated JSON marshal coverage tracked by #55.
Root cause
Existing service tests exercise API decoding paths, while the resource-level marshal round-trip tests skipped Attestation and AttestationsResponse.
Solution
Add marshal tests for empty and populated Attestation and AttestationsResponse values. The populated cases use the existing json.RawMessage comparator so Sigstore bundle JSON is compared semantically.
Tests run
Linked issue
Part of #55